Skip to content

Clarify concise Phabricator patch descriptions - #6834

Open
ayoubdiourin7 wants to merge 6 commits into
mozilla:masterfrom
ayoubdiourin7:issue-6727-concise-submit-patch-descriptions
Open

ayoubdiourin7 wants to merge 6 commits into
mozilla:masterfrom
ayoubdiourin7:issue-6727-concise-submit-patch-descriptions

Conversation

@ayoubdiourin7

Copy link
Copy Markdown
Contributor

Fixes #6727

@ayoubdiourin7
ayoubdiourin7 requested a review from a team as a code owner September 14, 2026 06:37

@suhaibmujahid suhaibmujahid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please rebase and solve the conflict?

Comment thread libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py Outdated
Comment thread libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py Outdated
Comment thread libs/hackbot-runtime/hackbot_runtime/actions/phabricator.py

@suhaibmujahid suhaibmujahid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this? Could you share and example for before and after?

@ayoubdiourin7

Copy link
Copy Markdown
Contributor Author

Yes, I tested it locally with the bug-fix agent on bug 2060318.

Recorded phabricator.submit_patch params:

Before
{
  "bug_id": 2060318,
  "title": "Bug 2060318 - Expunge private download files on shutdown too, so permanent private browsing honours browser.download.deletePrivate",
  "summary": "Private files queued via deletePrivateFileWhenPossible / deleteTemporaryPrivateFileWhenPossible were only removed on \"last-pb-context-exited\", which is never sent in permanent private browsing mode (and not when the browser quits with private windows still open). As a result, downloads were kept despite browser.download.deletePrivate being enabled.\n\nFactor the \"last-pb-context-exited\" cleanup into ExpungeAllPrivateFiles() and also call it on \"profile-before-change\". This keeps the existing enableDeletePrivate/deletePrivate pref check, as suggested in comment 5.",
  "test_plan": "Added the xpcshell test test_deletePrivate_on_shutdown in toolkit/components/downloads/test/unit/common_test_Download.js. It only fires \"profile-before-change\" and checks that:\n- a private download made with deletePrivate=true is deleted;\n- a launched temporary private download is deleted;\n- a private download made with deletePrivate=false is kept.\nPer the run instructions, I did not build this or run the test locally. It needs CI/try."
}
After
{
  "bug_id": 2060318,
  "title": "Bug 2060318 - Delete private download files on shutdown if the private browsing session never ended",
  "summary": "Private download files queued via deletePrivateFileWhenPossible / deleteTemporaryPrivateFileWhenPossible were only expunged on \"last-pb-context-exited\", which is never notified in permanent private browsing mode (nor if private windows are still open at shutdown). As a result, browser.download.deletePrivate had no effect in \"Never remember history\" mode.\n\nFactor the \"last-pb-context-exited\" cleanup into ExpungeAllPrivateFiles() and also call it on \"profile-before-change\". The lists are cleared after expunging, so calling it twice is harmless.",
  "test_plan": "New xpcshell test test_deletePrivate_on_shutdown in toolkit/components/downloads/test/unit/common_test_Download.js (run via test_DownloadCore.js and test_DownloadLegacy.js). It notifies only \"profile-before-change\" and checks that a deletePrivate private download and a launched temporary private download are removed, while a private download made with deletePrivate=false is kept.\n./mach xpcshell-test toolkit/components/downloads/test/unit/test_DownloadCore.js toolkit/components/downloads/test/unit/test_DownloadLegacy.js\nNot built or run in the automated environment (building was out of scope for this run)."
}

With this PR, the summary no longer points back to the bug discussion ("as suggested in comment 5") and gives a note that matters for review instead ("calling it twice is harmless").

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Commit messages written by Hackbot is very verbose

2 participants